Remove plugins from global & non-global plugins when they are removed from settings #3802
Conversation
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
|
Warning Rate limit exceeded@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 Walkthrough## Walkthrough
The internal logic of the `UninstallPluginAsync` method in `PluginManager.cs` was updated to ensure that, during plugin uninstallation, the plugin is also removed from the `GlobalPlugins` and `NonGlobalPlugins` collections, in addition to existing removal steps.
## Changes
| File(s) | Change Summary |
|------------------------------------------------|-----------------------------------------------------------------------------------------------|
| Flow.Launcher.Core/Plugin/PluginManager.cs | Enhanced `UninstallPluginAsync` to remove plugins from `GlobalPlugins` and `NonGlobalPlugins`. |
## Possibly related PRs
- #3786: Modified `UninstallPluginAsync` method signature and parameter handling, related to internal plugin removal logic.
- #3219: Enhanced public `UninstallPlugin` method for deleting plugin settings and user prompts, related to plugin uninstallation flow.
## Suggested reviewers
- jjw24
- taooceros✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
703-710: Good fix! Consider usingRemoveWherefor better performance and safety.The logic correctly addresses the issue by removing plugins from both
GlobalPluginsandNonGlobalPluginscollections during uninstallation. However, the implementation can be improved:Apply this diff to use
RemoveWhereforGlobalPluginswhich is more efficient and avoids potential iteration-while-modifying issues:- foreach (var globalPlugin in GlobalPlugins.Where(p => p.Metadata.ID == plugin.ID)) - { - GlobalPlugins.Remove(globalPlugin); - } + GlobalPlugins.RemoveWhere(p => p.Metadata.ID == plugin.ID);The current approach for
NonGlobalPluginsis already good as it collects keys first before removing them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/Plugin/PluginManager.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
- GitHub Check: Check Spelling
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Removes stale plugin references from global and non-global collections when a plugin is uninstalled, preventing unresponsive queries for removed plugins.
- Added removal of the plugin from
GlobalPlugins - Added cleanup of entries in
NonGlobalPlugins
Comments suppressed due to low confidence (1)
Flow.Launcher.Core/Plugin/PluginManager.cs:703
- [nitpick] Add or update unit tests to verify that
UninstallPluginAsyncalso removes the plugin fromGlobalPluginsto prevent regressions.
GlobalPlugins.RemoveWhere(p => p.Metadata.ID == plugin.ID);
Remove plugins from global & non-global plugins when they are removed from settings
2 participants
Remove plugins from global & non-global plugins when they are removed from settings
This can resolve the issue from @jjw24: